Skip to content

chore: refactor migration code and tests - #3222

Open
miparnisari wants to merge 4 commits into
mainfrom
refactor-migrations
Open

chore: refactor migration code and tests#3222
miparnisari wants to merge 4 commits into
mainfrom
refactor-migrations

Conversation

@miparnisari

@miparnisari miparnisari commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What this changes

Every datastore engine now gets a migration test. Migratable engines expose their migrations in order plus the earliest one the current code can run against.

The suite's new TestMigration walks an empty database one migration at a time, serving SpiceDB's schema service against the half-migrated database from that earliest migration onward — writing a schema before each remaining migration and re-reading it after, proving each migration preserves what the last one wrote. Out-of-repo engines get it by registering themselves.

To enable it, engines register their own builders. Each gains an enginebuilder.go registering with the new leaf package pkg/cmd/datastore/dsconfig, which now owns Config. pkg/cmd/datastore re-exports it and still links every in-repo engine: no caller changes imports, no behavior changes. Config had to move so the
test suite can build a datastore by engine name without depending on the command layer.

Migration plumbing also moved out of pkg/cmd/migrate.go into the exported pkg/datastore/migration.

Important files to review:

  • pkg/datastore/{test/migration.go,migration/migration.go},
  • pkg/cmd/datastore/{dsconfig/config.go,engines.go},
  • one enginebuilder.go.

@miparnisari miparnisari changed the title Refactor migrations chore: refactor migration code and tests Jul 10, 2026
@github-actions github-actions Bot added area/cli Affects the command line area/datastore Affects the storage system area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Jul 10, 2026
github-actions[bot]

This comment was marked as off-topic.

@miparnisari
miparnisari force-pushed the refactor-migrations branch 2 times, most recently from 31135a8 to 916899e Compare July 16, 2026 20:09
@miparnisari
miparnisari marked this pull request as ready for review July 16, 2026 20:11
@miparnisari
miparnisari requested a review from a team as a code owner July 16, 2026 20:11
ecordell
ecordell previously approved these changes Jul 16, 2026
Engine builders move from pkg/cmd/datastore into their engine packages,
registering themselves via the new leaf package pkg/cmd/datastore/dsconfig,
which also takes over the Config type and its options; pkg/cmd/datastore
re-exports everything, so callers are unaffected. Binaries link all in-repo
engines through a blank import of internal/datastore/engines.

This removes the static engine imports from the server-construction path,
so that shared test code can stand up a SpiceDB server without creating an
import cycle with the engines' own tests. Adding a new engine no longer
requires editing pkg/cmd/datastore.
@miparnisari
miparnisari force-pushed the refactor-migrations branch from 6ae38a5 to 95d4eb9 Compare July 29, 2026 22:03
Migratable engines can now list every migration they support, in order, and
declare the earliest migration the current code can operate against. The
datastore test suite's TestMigration uses this to apply every migration one
at a time from an empty database, standing up a SpiceDB server at the
earliest supported revision and verifying schema writes and reads through
its API across each remaining migration.
@miparnisari
miparnisari force-pushed the refactor-migrations branch from 95d4eb9 to 048185b Compare July 29, 2026 22:44
Callers of pkg/cmd/datastore.NewDatastore had to blank-import
internal/datastore/engines to use any built-in engine, which consumers of this
module could not do at all: the package is under internal/. The engine imports
move back into pkg/cmd/datastore, so importing it is once again enough, and
internal/datastore/engines and its blank imports are gone.

That was only possible because the datastore test suite reached the server
through pkg/cmd/server, so an engine's own tests could not import the suite
while pkg/cmd/datastore linked the engines. TestMigration now builds the
datastore from the engine's registered builder in the leaf package dsconfig and
serves SpiceDB's schema service in-process, keeping pkg/datastore/test free of
pkg/cmd/datastore and pkg/cmd/server.

Engines defined outside this repository keep registering themselves through
dsconfig.RegisterEngine and still get TestMigration from the suite.
memdb's tests and pkg/cmd/datastore's tests were moved to external test
packages to break import cycles that no longer exist now that
pkg/datastore/test does not reach the server through pkg/cmd.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cli Affects the command line area/datastore Affects the storage system area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) Skip-Changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants